Skip to content

Use libproc instead of lsof on Darwin to detect live gcroots - #16439

Open
booxter wants to merge 6 commits into
NixOS:masterfrom
booxter:gcroot-darwin-env
Open

booxter wants to merge 6 commits into
NixOS:masterfrom
booxter:gcroot-darwin-env

Conversation

@booxter

@booxter booxter commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

On Darwin, lsof was used to detect gcroots. This is bad:

  • lsof is very slow;
  • lsof can't look into process environment.

It may result in gcroots removed while e.g. nix shell or nixpkgs vm with 9p
store is running.

For the latter, see also:
NixOS/nixpkgs#440579

Context

This is not happening on Lix because they switched to libproc circa 2024.

This PR largely adopts Lix implementation. With libproc, we can now read
envvars and hold gcroots mentioned there.

The Lix code is taken largely verbatim but was adopted to this codebase where
necessary. In addition, two bugs were identified in their implementation. Those
bugs are fixed in separate commits, for easier review and reference of Lix
folks in case they ever want to backport them.

The bugs are:

  • empty argvs ("") were not properly handled, which may result in some envvars
    not consulted for possible gcroots;
  • if a thread exited during the scan, the whole pid may be skipped for gcroot
    search, which may result in removing active roots.

The PR re-enables a previously disabled test case on darwin.

lsof is left for other platforms. Also, - a drive-by change - /proc is no
longer crawled on non-Linux platforms since the specific /proc layout is not
portable. (The /proc walk did not fail, so it's only a minor optimization /
correctness fix.)

Fixes #3011
Fixes #13990


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions Bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Sep 8, 2026
@booxter

booxter commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Deployed it to MacOS 26 laptop (backported relevant patches from the series to latest from unstable). Then validated with the throw-away test script as follows: (it's produced by Codex)

#!/usr/bin/env bash

set -euo pipefail

testDir=$(mktemp -d)
holder=

cleanup() {
    if [[ -n "$holder" ]] && kill -0 "$holder" 2>/dev/null; then
        kill "$holder" 2>/dev/null || true
        wait "$holder" 2>/dev/null || true
    fi
    rm -f "$testDir/payload" "$testDir/hold"
    rmdir "$testDir" 2>/dev/null || true
}
trap cleanup EXIT

holderBash=$(command -v bash)
printf 'gc-runtime-test-%s-%s\n' "$$" "$RANDOM" > "$testDir/payload"
root=$(/run/current-system/sw/bin/nix store add-file "$testDir/payload")
mkfifo "$testDir/hold"

echo "active-nix=$(/run/current-system/sw/bin/nix eval --impure --raw --expr builtins.nixVersion)"
echo "holder-bash=$holderBash"
echo "disposable-path=$root"

# The empty argument exercises parsing of empty argv entries. The FIFO keeps
# this unentitled, Nix-built Bash process alive without starting a child that
# inherits GC_RUNTIME_ROOT.
env -i GC_RUNTIME_ROOT="$root" \
    "$holderBash" -c 'read -r value < "$1"' '' "$testDir/hold" &
holder=$!

sleep 1
kill -0 "$holder"

echo "delete-while-live:"
if /run/current-system/sw/bin/nix-store --delete "$root"; then
    echo "FAIL: Nix deleted a runtime-rooted path"
    exit 1
fi

test -e "$root"
echo "GOOD: deletion refused while holder was alive"

kill "$holder"
wait "$holder" 2>/dev/null || true
holder=

echo "delete-after-exit:"
/run/current-system/sw/bin/nix-store --delete "$root"
test ! -e "$root"

echo "PASS: Darwin environment runtime-root discovery works"
> /tmp/test-nix-darwin-runtime-root.sh
active-nix=2.35.2+5
holder-bash=/etc/profiles/per-user/ihrachyshka/bin/bash
disposable-path=/nix/store/xm749ai71r4v3dzcb67qwc1jxf72h4c2-payload
delete-while-live:
finding garbage collector roots...
deleting specified paths...
0 store paths deleted, 0.0 KiB freed
error: Cannot delete path '/nix/store/xm749ai71r4v3dzcb67qwc1jxf72h4c2-payload' since it is still alive. To find out why, use: nix-store --query --roots and nix-store --query --referrers
GOOD: deletion refused while holder was alive
delete-after-exit:
finding garbage collector roots...
deleting specified paths...
deleting '/nix/store/xm749ai71r4v3dzcb67qwc1jxf72h4c2-payload'
deleting unused links...

@booxter
booxter marked this pull request as ready for review September 8, 2026 01:01
@booxter

booxter commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

MacOS CI failure is in filetransfer-retry-backoff and looks unrelated.

@booxter booxter closed this Oct 1, 2026
@booxter booxter reopened this Oct 1, 2026
@github-project-automation github-project-automation Bot moved this from ⚖ Needs refinement to ✅ Done in Nix documentation Oct 1, 2026
booxter and others added 6 commits October 4, 2026 20:43
The runtime root scanner assumes the Linux procfs layout. Other
platforms already use lsof as their fallback, so avoid probing an
incompatible or optionally mounted procfs first.

Assisted-by: Codex (gpt-5.6-sol, high)
Darwin currently falls back to lsof, which cannot discover store paths
held only in process environments and is prohibitively slow. Use libproc
and KERN_PROCARGS2 to match the runtime-root coverage available on
Linux.

Based on Lix commits c03de0d, 1437d3d, and 068f4b1.

Fixes NixOS#3011.
Fixes NixOS#13990.

Co-authored-by: Artemis Tosini <me@artem.ist>
Co-authored-by: Jade Lovelace <lix@jade.fyi>
Co-authored-by: Lily Ballard <lily@ballards.net>
Assisted-by: Codex (gpt-5.6-sol, high)
The libproc implementation provides the process, environment, and
open-file root discovery covered by this test.

Assisted-by: Codex (gpt-5.6-sol, high)
KERN_PROCARGS2 represents empty arguments as consecutive NUL bytes. Skip
padding only after the executable, then advance exactly one entry per
argument so environment roots are not missed.

Strengthen gc-runtime to keep the environment root out of the GC process
and cover an empty argument.

Assisted-by: Codex (gpt-5.6-sol, high)
A thread may exit between listing it and querying its working directory.
Ignore ESRCH for that thread so the remaining thread roots are still
inspected.

This is similar to how we gracefully continue when an fd is closed.

Assisted-by: Codex (gpt-5.6-sol, high)
Document the new libproc scanner and the macOS restriction on reading
environment variables from entitled processes.

Assisted-by: Codex (gpt-5.6-sol, high)
@booxter
booxter force-pushed the gcroot-darwin-env branch from 04e456c to 938c019 Compare October 5, 2026 00:43
Comment on lines +99 to +100
// and has been available since OS X 10.10, but is not exposed
// by the SDK headers. Its numeric value is 22.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's safe to drop this hack then since we are in the clear by having nixpkgs only support 14 and upwards?

Comment on lines +183 to +185
if (e.is(std::errc::no_such_file_or_directory) || e.is(std::errc::no_such_process)
|| e.is(std::errc::invalid_argument) || e.is(std::errc::permission_denied)
|| e.is(std::errc::operation_not_permitted) || e.is(std::errc::io_error))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe extend the is overload set with a overload that takes a variadic template pack?


tids.resize(tidBufSize / sizeof(uint64_t));
for (auto tid : tids) {
struct proc_threadwithpathinfo threadPathInfo;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why we need the C-style struct suffix? Seems like it could be safely dropped. I'd also maybe opt for explicitly qualifying with ::proc_threadwithpathinfo but that's more of a keyboard heating thing than an actual issue in practice.

@xokdvium xokdvium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing this. Haven't yet tested this on my mac, but looks promising indeed. Mostly stylistic questions.

Comment on lines +55 to +76
// File descriptors.
std::vector<struct proc_fdinfo> fds;
std::size_t fdBufSize = 1;
while (fdBufSize > fds.size() * sizeof(struct proc_fdinfo)) {
// Reserve some extra size so we don't fail too much.
fds.resize((fdBufSize + fdBufSize / 8) / sizeof(struct proc_fdinfo));
errno = 0;
auto size = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, fds.data(), fds.size() * sizeof(struct proc_fdinfo));

// The libproc wrapper converts a -1 syscall result to 0. A
// process with no file descriptors also returns 0, but leaves
// errno unchanged.
if (size <= 0) {
if (errno == 0) {
fdBufSize = 0;
break;
}
throw SysError("listing pid %1% file descriptors", pid);
}
fdBufSize = size;
}
fds.resize(fdBufSize / sizeof(struct proc_fdinfo));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious. I guess we could use this stuff in closeExtraFDs in process spawning machinery instead of the less efficient approach that it takes now.

return boost::regex(quotedStoreDir + R"(/[0-9a-z]+[0-9a-zA-Z\+\-\._\?=]*)");
}

void findDarwinRuntimeRoots(const StoreDirConfig & config, UncheckedRoots & unchecked);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in the future we could also factor out Linux-specific code. Seems nice, though I'd probably rename this to something like findPlatformRuntimeRoots so that we could just have separate platform-specific files implementing this function.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation with-tests Issues related to testing. PRs with tests have some priority

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

On Darwin, garbage collector may remove paths that are listed in an environment variable nix-collect-garbage is slow on OS X because of lsof

2 participants